Skip to content

fix(desktop): surface profile-switch failures instead of falling back to primary socket - #81165

Closed
echoes666 wants to merge 2 commits into
NousResearch:mainfrom
echoes666:fix/profile-switch-v2
Closed

echoes666 wants to merge 2 commits into
NousResearch:mainfrom
echoes666:fix/profile-switch-v2

Conversation

@echoes666

Copy link
Copy Markdown

Fixes #81094

Problem

When switching to a secondary profile, openSecondary / ensureGatewayForProfile could silently fall back to the primary socket if the target backend's WebSocket failed to open (e.g. a manually started gateway process holding the profile's resources). This routed the user's messages to the wrong profile's backend and caused cross-profile session writes.

Changes

  • apps/desktop/src/store/gateway.ts (openSecondary): rethrow connect failures with an actionable error message instead of letting the caller's catch path fall through to the primary socket.
  • apps/desktop/src/store/profile.ts (ensureGatewayProfile): log and rethrow the switch failure instead of silently resetting the swap target.

Why fail loudly instead of retrying?

A silent fallback is strictly worse than an explicit error here: the user's message would be persisted into the wrong profile's session. Failing loudly surfaces the conflict (e.g. a manually started gateway holding the profile) and the message suggests how to resolve it. Retrying against a manually held socket would loop forever.

Notes

Replaces the previous PR #81099 (same fix, rebased onto current main; the old branch was force-pushed during a botched amend and GitHub does not allow reopening it).

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) area/profiles Multi-profile isolation, HERMES_HOME scoping sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 7, 2026
@spfcraze

spfcraze commented Aug 8, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary:
openSecondary's new rethrow is consumed by the unchanged catch { scheduleReconnect(entry) } at apps/desktop/src/store/gateway.ts:296, so a failed profile switch still ends at setActive(key) with a closed socket and the error never reaches the new catch in ensureGatewayProfile.

Problems:

  • ensureGatewayForProfile wraps await openSecondary(entry) in a bare catch { scheduleReconnect(entry) } (apps/desktop/src/store/gateway.ts:295-297) that this diff does not touch — the rethrown Failed to connect to profile ... error is discarded there, and execution continues to setActive(key) (gateway.ts:301) with the secondary socket still closed.
  • The new catch (error) in ensureGatewayProfile (apps/desktop/src/store/profile.ts) fires only if ensureGatewayForProfile or syncConnectionToActiveProfile rejects; on the connect-failure path the PR description names, ensureGatewayForProfile resolves normally, so the switch proceeds exactly as on main and the new error message is not shown. A gatewaySwitch rejection already propagated through main's finally-only try, so the catch's only new effect is the console.error.

Solution:
Make ensureGatewayForProfile propagate the failure — rethrow from its catch (or record the failure and skip setActive(key)) — so the error message built in openSecondary reaches a caller that can surface it; as written, every call site of openSecondary (reconnectSecondary, openGatewayForProfile, ensureGatewayForProfile) discards the error.


Checked against d399455 — the tip of fix/profile-switch-v2 when this was written — and b3aa561, main at the same moment.

@echoes666

Copy link
Copy Markdown
Author

Thanks for catching this — you're right, the rethrow was swallowed by the unchanged catch { scheduleReconnect(entry) } in ensureGatewayForProfile, so a failed switch still fell through to setActive(key) with a closed socket and the new catch in ensureGatewayProfile never fired. The original fix was ineffective.

Fixed in 0882362b2 (pushed to this branch): the catch now keeps the reconnect schedule (transient failures still self-heal via the existing backoff) but re-throws, so ensureGatewayForProfile aborts before setActive, and ensureGatewayProfile receives the error and surfaces it instead of silently activating a profile whose backend is unreachable.

Verified: tsc --noEmit clean; profile.test.ts + gateway-switch.test.ts (12 tests) pass.

@teknium1

Copy link
Copy Markdown
Collaborator

Partial overlap with #87600, now on main: the silent-misroute half is fixed (activeGateway() returns null for missing named scopes; eviction paths restore the primary explicitly). What SURVIVES here and remains wanted: the error-surfacing UX — main's ensureGatewayForProfile still does catch { scheduleReconnect(entry) } then setActive, silently activating a scope whose socket is dead; no error reaches the user. Your revised rethrow+surface commit addresses exactly that gap. A rebase onto the post-#87600 store would slim this PR down to the UX half — happy to review it in that shape.

…y activating

Rebase of NousResearch#81165 onto post-NousResearch#87600 main, slimmed to the error-surfacing UX
half as requested - the silent-misroute half already landed via NousResearch#87600.

- ensureGatewayForProfile: keep scheduleReconnect() on a failed secondary
  dial (transient failures still self-heal) but rethrow so the caller can
  surface the failure instead of falling through to setActive() with a
  closed socket (NousResearch#81094). openSecondary logs the dial target and rethrows
  the ORIGINAL error so reconnectSecondary's message-based fail-stop
  classification ("No connection with id", "no longer exists") keeps working.
- profile.ensureGatewayProfile: propagate the rejection to await-callers
  (session actions, slash commands surface it in their own flows); the three
  fire-and-forget call sites (selectProfile, newSessionInProfile, voice
  wiring) get explicit .catch(notifyError) so the failure is always visible.
- Tests: gateway.test.ts pins rethrow-without-activation plus backoff
  self-heal; profile-switch-failure.test.ts pins the profile-door
  propagation; gateway-shared-remote's pooled-reconnect test updated to
  the new contract (reject first, publish once the backend returns).

Verified: tsc --noEmit clean; 65 tests across the gateway*/profile*
suites pass; eslint and prettier clean on touched files.
@echoes666
echoes666 force-pushed the fix/profile-switch-v2 branch from db75f17 to f748f56 Compare August 22, 2026 10:21
@addelh

addelh commented Aug 22, 2026

Copy link
Copy Markdown

The rebase targets the surviving UX seam cleanly, but the new rethrow skips the activation-lease cleanup immediately below this block. On current main, entry.activationLeaseUntil = 0 is reached only because the catch swallows the dial error; after this change, every rejected profile-door dial leaves the failed entry leased against pruning for the full 30-second window. That is bounded, but it defeats the documented “activation is settling either way — release the prune lease” contract and can retain a failed/cancelled target after the switch has already surfaced as failed.

Please release the lease on both success and rejection (for example, put the dial block and lease reset under try/finally, while still scheduling reconnect and rethrowing the original error), and add a regression assertion for the rejected path. The existing fake-timer test proves reconnect remains armed, but not that the activation lease is released.

Clear the profile-door activation lease in a finally block whether the
secondary dial succeeds or rejects. Preserve reconnect scheduling and the
original error propagation.

Add a regression proving a rejected activation can be pruned immediately.
@echoes666

Copy link
Copy Markdown
Author

Addressed the rejected-dial lease cleanup in the latest commit:

  • The profile-door dial block now releases activationLeaseUntil from finally on both success and rejection.
  • The reconnect catch remains scoped to openSecondary(), so only an actual dial failure schedules reconnect and the original error still propagates.
  • Added a regression that rejects the first dial, immediately prunes the failed entry, and verifies disposal; the existing reconnect and no-activation assertions remain intact.

Validation:

  • gateway.test.ts: 4/4 passed
  • related gateway/profile suites: 13 files, 72/72 passed
  • Desktop typecheck: passed
  • Prettier and ESLint on the touched files: passed

@addelh

addelh commented Aug 23, 2026

Copy link
Copy Markdown

Verified the lease-cleanup follow-up at bf6fca40e:

  • the try/finally now clears activationLeaseUntil on both success and rejection while preserving reconnect scheduling and the original error;
  • the new rejected-first-dial test proves the failed entry is immediately pruneable/disposed;
  • targeted UI suites: 8/8 passed (gateway.test.ts, profile-switch-failure.test.ts, gateway-shared-remote.test.ts);
  • Desktop typecheck: passed.

That resolves my review concern. GitHub currently reports the PR as CONFLICTING against main, so this is now an actual-conflict case where rebasing/updating the contributor branch is warranted; after that, the remaining owner is maintainer review/CI.

teknium1 pushed a commit that referenced this pull request Aug 26, 2026
…y activating

Rebase of #81165 onto post-#87600 main, slimmed to the error-surfacing UX
half as requested - the silent-misroute half already landed via #87600.

- ensureGatewayForProfile: keep scheduleReconnect() on a failed secondary
  dial (transient failures still self-heal) but rethrow so the caller can
  surface the failure instead of falling through to setActive() with a
  closed socket (#81094). openSecondary logs the dial target and rethrows
  the ORIGINAL error so reconnectSecondary's message-based fail-stop
  classification ("No connection with id", "no longer exists") keeps working.
- profile.ensureGatewayProfile: propagate the rejection to await-callers
  (session actions, slash commands surface it in their own flows); the three
  fire-and-forget call sites (selectProfile, newSessionInProfile, voice
  wiring) get explicit .catch(notifyError) so the failure is always visible.
- Tests: gateway.test.ts pins rethrow-without-activation plus backoff
  self-heal; profile-switch-failure.test.ts pins the profile-door
  propagation; gateway-shared-remote's pooled-reconnect test updated to
  the new contract (reject first, publish once the backend returns).

Verified: tsc --noEmit clean; 65 tests across the gateway*/profile*
suites pass; eslint and prettier clean on touched files.
teknium1 added a commit that referenced this pull request Aug 26, 2026
teknium1 added a commit that referenced this pull request Aug 26, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Merged via #95080 at 682a34a — your commit was cherry-picked into the consolidated salvage PR with authorship preserved, so this work carries your name in git history.

Thanks for surfacing profile-switch failures instead of silently falling back to the primary socket — failing loudly here was the right call.

Closing this original PR now that the consolidated branch has landed on main. Thanks for contributing to the desktop multi-gateway campaign (tracker: #94724).

@teknium1 teknium1 closed this Aug 26, 2026
and7777 pushed a commit to and7777/hermes-agent that referenced this pull request Aug 27, 2026
…y activating

Rebase of NousResearch#81165 onto post-NousResearch#87600 main, slimmed to the error-surfacing UX
half as requested - the silent-misroute half already landed via NousResearch#87600.

- ensureGatewayForProfile: keep scheduleReconnect() on a failed secondary
  dial (transient failures still self-heal) but rethrow so the caller can
  surface the failure instead of falling through to setActive() with a
  closed socket (NousResearch#81094). openSecondary logs the dial target and rethrows
  the ORIGINAL error so reconnectSecondary's message-based fail-stop
  classification ("No connection with id", "no longer exists") keeps working.
- profile.ensureGatewayProfile: propagate the rejection to await-callers
  (session actions, slash commands surface it in their own flows); the three
  fire-and-forget call sites (selectProfile, newSessionInProfile, voice
  wiring) get explicit .catch(notifyError) so the failure is always visible.
- Tests: gateway.test.ts pins rethrow-without-activation plus backoff
  self-heal; profile-switch-failure.test.ts pins the profile-door
  propagation; gateway-shared-remote's pooled-reconnect test updated to
  the new contract (reject first, publish once the backend returns).

Verified: tsc --noEmit clean; 65 tests across the gateway*/profile*
suites pass; eslint and prettier clean on touched files.
and7777 pushed a commit to and7777/hermes-agent that referenced this pull request Aug 27, 2026
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…y activating

Rebase of NousResearch#81165 onto post-NousResearch#87600 main, slimmed to the error-surfacing UX
half as requested - the silent-misroute half already landed via NousResearch#87600.

- ensureGatewayForProfile: keep scheduleReconnect() on a failed secondary
  dial (transient failures still self-heal) but rethrow so the caller can
  surface the failure instead of falling through to setActive() with a
  closed socket (NousResearch#81094). openSecondary logs the dial target and rethrows
  the ORIGINAL error so reconnectSecondary's message-based fail-stop
  classification ("No connection with id", "no longer exists") keeps working.
- profile.ensureGatewayProfile: propagate the rejection to await-callers
  (session actions, slash commands surface it in their own flows); the three
  fire-and-forget call sites (selectProfile, newSessionInProfile, voice
  wiring) get explicit .catch(notifyError) so the failure is always visible.
- Tests: gateway.test.ts pins rethrow-without-activation plus backoff
  self-heal; profile-switch-failure.test.ts pins the profile-door
  propagation; gateway-shared-remote's pooled-reconnect test updated to
  the new contract (reject first, publish once the backend returns).

Verified: tsc --noEmit clean; 65 tests across the gateway*/profile*
suites pass; eslint and prettier clean on touched files.
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Desktop: switching to a secondary profile can route messages to the primary backend when a manual gateway process exists

5 participants